New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
[New] Component detection: add util.isReactHookCall
#3156
[New] Component detection: add util.isReactHookCall
#3156
Conversation
Rename Components test suite filename to match sibling lib/util/Components filename. Extend Components testComponentsDetect function to accept custom instructions, and to accumulate the results of processing those instructions. Add utility to check whether a CallExpression is a React hook call.
4144ea4
to
8232873
Compare
Codecov Report
@@ Coverage Diff @@
## master #3156 +/- ##
=======================================
Coverage 97.51% 97.51%
=======================================
Files 119 119
Lines 8130 8180 +50
Branches 2895 2934 +39
=======================================
+ Hits 7928 7977 +49
- Misses 202 203 +1
Continue to review full report at Codecov.
|
I renamed the test file in the default branch so when you rebase it'll show the diff properly :-) |
3ef8aba
to
b590a60
Compare
const augmentedInstructions = fromEntries( | ||
entries(instructions || {}).map((nodeTypeAndHandler) => { | ||
const nodeType = nodeTypeAndHandler[0]; | ||
const handler = nodeTypeAndHandler[1]; | ||
return [nodeType, (node) => { | ||
instructionResults.push({ type: nodeType, result: handler(node, context, components, util) }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm a bit confused about these changes; could you walk me through why they're needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of the Components
test scaffolding is to allow exfiltration of the Components.detect
locals to a scope in the test where assert
is available, but without having to write a complete rule for every test with its own Program:exit
clause.
My original implementation exfiltrated the Components.detect
locals, but didn't make it easy to actually test some of those locals; in particular I couldn't test utils
because the utility functions expect AST nodes, and by the time Program:exit
fires, I've lost the CallExpression
nodes I need.
These changes are intended to aid in writing new test cases for Components
functionality; specifically, for testing utils
functionality whose methods expect many different AST node types besides CallExpression
.
Custom instructions passed into the testComponentsDetect
get augmented and then incorporated into a custom rule, and when that rule runs over code the augmented instructions do two things:
- Provide the
Components.detect
locals to the instruction callback. Ordinarily the instruction callback only receives anode
as an argument and relies on having been defined in theComponents.detect
closure to access the special locals it provides. - Accumulate a log in
instructionResults
as the visitor traverses the parsed AST; the log tracks which node type was traversed, and the result of that visit.
Today
Taken together, these allow creation of test cases intended to read something like:
The visitor visited a
CallExpression
node
The visitor had access to theutil
local
Theutil
functionisReactHookCall
returned the following result for the visitedCallExpression
node
Tomorrow
Down the road, as we work to flesh out coverage for the code in Components
, we can write new tests cases.
The visitor visited a
ClassDeclaration
The visitor had access to theutil
local
Theutil
functionisES6Component
returned the following result for the visitedClassDeclaration
node
|
||
return Object.assign({}, augmentedInstructions, { | ||
'Program:exit'(node) { | ||
if (augmentedInstructions['Program:exit']) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't yet exercised, but I figured if someone needed to test util
functionality when handling the Program:exit
instruction it should behave the same way as handling any other instruction type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a self-test section to the test/util/Components
file in order to exercise the logging behavior while visiting Program:exit
instructions.
e9dd937
to
2f212c6
Compare
util.isReactHookCall
f06944d
to
5a25380
Compare
testComponentsDetect
function to accept custom instructions, and to accumulate the results of processing those instructions.CallExpression
is a React hook call.馃棧 Discussion
This work extracts the somewhat complicated guard clause from #2921
It also makes hooks-usage detection generic, so the util can be used to check for:
useState
calls in [New] Symmetric useState hook variable names聽#2921I converted this PR to a Draft while I work to extract this in a way that leaves the
hook-use-state
rule clean and easy to process.